Dear reviewers, please review 😄

Codecov Report

Merging #1118 (2fb9cc0) into main (7fa450d) will increase coverage by 0.04%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1118      +/-   ##
==========================================
+ Coverage   89.70%   89.75%   +0.04%     
==========================================
  Files         106      106              
  Lines       16364    16371       +7     
  Branches       35       35              
==========================================
+ Hits        14680    14693      +13     
+ Misses       1677     1671       -6     
  Partials        7        7              

CodSpeed Performance Report

Merging #1118 will not alter performance

_{Comparing akx:expose-pydantic-errors-omit-url (2fb9cc0) with main (7fa450d)}

Summary

✅ 140 untouched benchmarks

It looks like the Codspeed benchmark is a bit flaky – pushing non-code changes made it oscillate between "perf bad" and "this is fine".

@akx thanks for this proposal. The Codspeed benchmark does indeed have some volatility at the moment, which we hope to improve with #1085

Regarding the feature added here, if I recall correctly @samuelcolvin, @adriangb and myself were discussing something very similar just last week. I think we came to the conclusion that having this as a global setting had some drawbacks.

As an alternative, we wondered if it would make sense to have this as a property on errors, so on each ValidationError you could write e.show_url_in_repr = False (or True) to override the environment variable for that specific error. Would that be suitable for your use case?

I think we came to the conclusion that having this as a global setting had some drawbacks.

It kind of already is a global setting, namely the undocumented environment variable. 😁
I agree global state isn't generally a great idea, though...

[...] on each ValidationError you could write e.show_url_in_repr = False (or True) to override the environment variable for that specific error. Would that be suitable for your use case?

That means I'd have to look through all of my codebase (and dependencies that might use Pydantic), and set the property before anything happens to repr() the error, which sounds pretty tedious.

There's something of a precedent in the form of hide_input_in_errors, but a hide_url_in_errors would also need to be set on each model by hand.

It kind of already is a global setting, namely the undocumented environment variable. 😁

Yes, but an environment variable which we read once rather than mutable state.

That means I'd have to look through all of my codebase (and dependencies that might use Pydantic), and set the property before anything happens to repr() the error, which sounds pretty tedious.

Potentially, what's your use case which means that the environment variable at startup isn't useful to you?

Yes, but an environment variable which we read once rather than mutable state.

If the issue is the possible perf loss from having to read the GILProtected value once per ValidationError.__repr__ call, I'm sure that could be mitigated (e.g. making the value a regular atomic bool).

Potentially, what's your use case which means that the environment variable at startup isn't useful to you?

The environment variable works for me personally, but if the canonical way to disable the URLs app-wide is the envvar, then it should be at least documented 😄

The environment variable works for me personally, but if the canonical way to disable the URLs app-wide is the envvar, then it should be at least documented 😄

I asked @samuelcolvin about this earlier today, agreed we should make this public and documented but would prefer to flip it around to have PYDANTIC_ERRORS_INCLUDE_URL=True/False (to match the argument which is include_url)

I asked @samuelcolvin about this earlier today, agreed we should make this public and documented but would prefer to flip it around to have PYDANTIC_ERRORS_INCLUDE_URL=True/False (to match the argument which is include_url)

I can look into making a PR implementing that instead today :)

Emdraftened this for the time being.

Superseded by #1123